-
Notifications
You must be signed in to change notification settings - Fork 241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Native Send Component #1874
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
bf2eb1f
to
bb6562d
Compare
342bbfb
to
95ee15c
Compare
95ee15c
to
43a0f8a
Compare
c444463
to
353a8e8
Compare
showSend, | ||
setShowSend, | ||
isSendClosing, | ||
setIsSendClosing, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could probably simplify these, since you only ever show a single thing at a time, only really need a single isClosing and setIsClosing, probably possible to also have a single show: 'qr' | 'send' | 'swap' and setShow(type: 'qr' | 'send' | 'swap'), would make adding additional types easier.
const { tokenBalances } = useWalletAdvancedContext(); | ||
const context = useSendContext(); | ||
|
||
const walletHasEth = context.isInitialized && context.ethBalance > 0.000001; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why 0.000001? may want to extract to a const, MIN_ETH_TO_SEND
setValidatedInput, | ||
handleRecipientInputChange, | ||
className, | ||
}: AddressInputProps) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: SendAddressInputProps
selectedRecipientAddress: RecipientAddress; | ||
recipientInput: string; | ||
setRecipientInput: Dispatch<SetStateAction<string>>; | ||
setValidatedInput: Dispatch<SetStateAction<RecipientAddress>>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the intention by passing props is to allow this to be reusable, would be better to make these standard callbacks vs. setState callbacks
inputMode="text" | ||
placeholder="Basename, ENS, or Address" | ||
value={displayValue} | ||
inputValidator={() => !!validateAddressInput(recipientInput)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inputValidator={(value) => !!validateAddressInput(value)}
cryptoAmount={cryptoAmount ?? ''} | ||
exchangeRate={exchangeRate} | ||
exchangeRateLoading={false} | ||
currency={'USD'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: unnecessary {}
address: AddressType | null; | ||
senderChain: Chain | null | undefined; | ||
handleClick: () => Promise<void>; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
className?
fiatAmount={fiatAmount ?? ''} | ||
cryptoAmount={cryptoAmount ?? ''} | ||
asset={selectedToken?.symbol ?? ''} | ||
currency={'USD'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: unnecessary {}
What changed? Why?
Notes to reviewers
How has it been tested?